Skip to content

Naive fix for #1413 #1414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Naive fix for #1413 #1414

wants to merge 2 commits into from

Conversation

ccustine
Copy link

@ccustine ccustine commented Mar 5, 2015

I have been running this for a few days to get go application run configs to work. I'm sure this could be done much more elegantly and the run configs seem ready for a refactoring but this works for me so I thought it might be useful. I have some other comments for run configs that I will add directly to the thread on #1413.

@dlsniper dlsniper added this to the 1.0.0 milestone Mar 5, 2015
@@ -194,4 +195,14 @@ public String getWorkingDirectory() {
public void setWorkingDirectory(@NotNull String workingDirectory) {
myWorkingDirectory = workingDirectory;
}

@NotNull
public String getFilePath() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need it in base class? Why do you need duplicates of this methods in GoTestRunConfiguration?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change to move filePath up to the base is probably unnecessary now. I had originally used this to point to the app root directory until I looked up the source root and eliminated the file path from the run app config entirely. Also, I totally overlooked the TestRun configs so I will fix these tomorrow.

Like I said, totally naive approach and I have never done Idea plugin work before so there are probably much better ways to do this later.

@zolotov
Copy link
Contributor

zolotov commented Mar 5, 2015

Thank you for help. I see that this is a naive fix, but current implementation is quite naive too and I don't see any reasons fix something in naive way and break other things that works in already implemented naive approach.
I like your point in #1413 about something like the Java Application run config, if you have particular thoughts about it (like how it should look like, what user should be able to configurate, how to run go build depends on user's configuration and so on), please share them with me.

@ccustine
Copy link
Author

ccustine commented Mar 5, 2015

@zolotov Thanks for the comments, and you are absolutely correct. I think the main thing is that to succesfully go build -o <tempxecutable> it seems that you have to run in top package directory with no other arguments (for instance go build -o tmpexe ./... and go build -o tmpexe main.go fail for different reasons) so go build -o tmpexe is the only way I have found to make that work. My attempt to discover the root based on the selected module was just a hack as I try to learn the plugin sdk :-)

One thing that I have not experimented with is having multiple Go modules in a single project and seeing if the run configurations will work. I assume this should be supported eventually so I will try to verify soon.

I'm really interested to get all of these scenarios working properly because I have projects with all 3 variations of single main() in file, pkg main and main() in application, and the sub directory approach to getting multiple executables. I will give it some more thought as I learn this plugin sdk and see what I can come up with.

@zolotov
Copy link
Contributor

zolotov commented Mar 5, 2015

@ccustine thanks, looking forward to your comments. It's really great that you have experience with different workflows in go, because you know most of main contributors are not really go-guys so we have huge lack in understanding workflow questions.

About SDK, instead of source roots in this plugin we usually operate with following entities:
– sdk ($GOROOT), that you can retrieve with GoSdkService.getInstance().getSdkHomePath()
– libraries ($GOPATH), that you can get their 'src' directories with GoSdkUtil.getGoPathsSources
– importpaths (GoFile.getImportPath() will return path to file relative to $GOPATH or $GOROOT) and package (GoFile.getPackageName())

Also GoSdkUtil contains some handy methods, maybe you'll find them helpful.

@ccustine
Copy link
Author

ccustine commented Mar 9, 2015

I'm going to close this PR for now. I am working on another approach which should be better and I will just open a new PR because it might be a few days.

@ccustine ccustine closed this Mar 9, 2015
@zolotov
Copy link
Contributor

zolotov commented Mar 9, 2015

I am working on another approach which should be better and I will just open a new PR because it might be a few days.

Looking forward to it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants